Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Experiment with kotlinpoet without builders #487

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Experiment with kotlinpoet without builders #487

wants to merge 1 commit into from

Conversation

jmfayard
Copy link

I thought the builder pattern was mostly useless in kotlin,
which has named parameters, default values
and works well with an immutable functional style like the one used by kotlinpoet.

This is an experiment with the kotlinpoet codebase to test this hypothesis.

Of course I don't advocate removing the builders apis,
that would be bad for people calling kotlinpoet from java or with an existing code base.

I'm only adding stuff, just enough to see how that would like for the README,
I'm delegating the actual work to the builders.
even adding a configuration lambda for all the corner cases where a builder can be useful

I modified the examples in the README to see what the changes would look like

I thought the builder pattern was mostly useless in kotlin,
which has named parameters, default values and works well
with an immutable functional style like the one used by kotlin poet.

This is an experiment with the kotlinpoet codebase to test this hypothesis.

Of course I don't advocate removing the builders apis,
that would be bad for people calling kotlinpoet from java or with an existing code base.
I'm only adding stuff, and delegating to those builders,
even adding a configuration lambda for all the corner cases where a builder can be useful

I modified the examples in the README to see what the changes would look like
@Egorand
Copy link
Collaborator

Egorand commented Nov 29, 2018

We still feel that builders are the most flexible solution for KotlinPoet API, and hosting two different API styles in the library core would make it hard to maintain and consume. Nevertheless, I'm happy to keep this PR around in case someone borrows your idea to build their own wrapper on top of KotlinPoet API, if this is what fits certain use-cases better. Thanks for building this, and sorry for taking that long to respond.

@jmfayard
Copy link
Author

@Egorand no problem!

@jmfayard
Copy link
Author

jmfayard commented Nov 30, 2018

@ergoland actually I can give you more feedback on this: I wrote a Gradle plugin based on Kotlinpoet recentlly.
The builders were not a problem once I was into it. If that's the price to make the library compatible with Java, I'm totally happy with it.

On the other hand, I don't like what you need to do to declare a simple class with properties declared in the constructor.

IMHO Kotlinpoet nailed only the second half of principle: "Make simple things easy and make complicated things possible"

@Egorand
Copy link
Collaborator

Egorand commented Nov 30, 2018

Thanks for the feedback! The philosophy of KotlinPoet is that we don't model things that are just Kotlin's syntactic sugar, instead we require the full model to be flexible in how we emit it. Primary constructor properties are an example of this. inline properties are another example - what you really do when you declare a property as inline is making both of its accessors inline, hence with KotlinPoet you'll have to mark the getter and the setter with KModifier.INLINE, and in that case we'll emit the property with the inline modifier.

@Egorand Egorand changed the base branch from master to main July 5, 2023 09:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants